fix(editor): restore scene auto-framing on load - #766
Conversation
|
I hit an error while handling your request (Model unavailable on AI Gateway free tier: Free tier users do not have access to this model. Upgrade to paid credits at https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%3Fmodal%3Dtop-up for unrestricted…). Please try again, rephrase, or reach out if it keeps failing. Error id: 67426f0f-caa3-4001-a1cd-88e8b4ac5c0a |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Bugbot on pascalorg#766: the level-follow early return treated every same-id rerun and every null->level as a no-op. Keep the first auto-select skip so auto-frame still owns load, but follow exploded/stacked Y changes and a level pick after building/breadcrumb/resetSelection.
Bugbot on pascalorg#766: skipping the first null->level in the controls lifetime also dropped a real pick when load restored site phase (levelId null). Keep first-load auto-frame / empty-scene default pose; after that, pan on level or levelMode changes. Y-idempotence still swallows no-ops.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 38f224f. Configure here.
Aymericr
left a comment
Aymericr
left a comment
There was a problem hiding this comment.
There is still a blocking lifecycle case in the current head: if the scene becomes ready while first-person mode is active, the new fit-scene emit is ignored, and the level-follow effect returns before consuming its first-load state. When orbit controls resume, there is no later guaranteed fit, so a populated scene can remain on the default/unframed camera. The source-string test does not exercise this state transition.
Please cover scene-ready-during-first-person followed by returning to orbit with a behavioral test, and make the pending frame survive until orbit controls can apply it. I’m marking this for human product review as well because this changes load framing, level-follow, and the production exposure of window.__pascalCameraControls together.
…obbering it useAutoFrame was accidentally removed in e688792, so nothing emitted camera-controls:fit-scene on load; the level-follow effect's first-run default pose then reset the camera after framing on fast client-side navigations. Restore the hook, gate the default pose to scene-less editors, skip the initial null->level transition, and re-emit fit-scene once the viewer signals scene-ready. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Assert EditorContent mounts useAutoFrame and re-emits fit-scene on viewer scene-ready, and that CustomCameraControls keeps the __pascalCameraControls helper ungated by NODE_ENV.
Bugbot on pascalorg#766: the level-follow early return treated every same-id rerun and every null->level as a no-op. Keep the first auto-select skip so auto-frame still owns load, but follow exploded/stacked Y changes and a level pick after building/breadcrumb/resetSelection.
Bugbot on pascalorg#766: skipping the first null->level in the controls lifetime also dropped a real pick when load restored site phase (levelId null). Keep first-load auto-frame / empty-scene default pose; after that, pan on level or levelMode changes. Y-idempotence still swallows no-ops.
Queue fit-scene while orbit controls are unmounted (first-person or pre-ref), flush on orbit resume, and cover the FP->orbit transition with a behavioral helper test for Aymericr's pascalorg#766 review.
38f224f to
e7adddc
Compare
|
Thanks for the review, Aymericr, really helpful catch. Addressed the first-person / scene-ready case: Rebased onto latest |
2 participants
What does this PR do?
Restores scene auto-framing on load and stops the level-follow effect from clobbering the framed camera pose.
useAutoFrame()inEditorContentagain (it was dropped in an earlier refactor)camera-controls:fit-sceneso fast client navigations keep the framed posewindow.__pascalCameraControlswithout aNODE_ENVgate (client Turbopack leavesprocessundefined)How to test
bun test packages/editor/src/hooks/use-auto-frame.wiring.test.ts— all passbun run check packages/editor/src/hooks/use-auto-frame.wiring.test.ts packages/editor/src/components/editor/custom-camera-controls.tsx packages/editor/src/components/editor/index.tsxbun dev: open a non-empty scene and confirm the camera frames the content instead of a black / default distant poseScreenshots / screen recording
N/A for the unit path (wiring + controls). Optional recording of load framing welcome if reviewers want visual proof.
Checklist
bun devbun checkto verify)mainbranchNote
Medium Risk
Changes load-time camera orchestration and level-follow timing; regressions would show wrong framing or fighting camera poses, but no security or data paths are touched.
Overview
Restores automatic orbit framing when a scene loads by wiring
useAutoFrame()back into the editor and emittingcamera-controls:fit-sceneagain once the viewer reports scene-ready (using XZ bounds), so fast client navigations are not overridden by other camera effects.Refactors fit-scene handling into
fit-scene-framing.ts(computeFitSceneLookAt, queue/apply/ignore plans).CustomCameraControlsqueues pending fits while first-person mode or an unmounted orbit ref is active, then flushes after orbit resumes (includingrequestAnimationFrameretries).Adjusts level-follow camera behavior so the default
(20,20,20)look-at runs only on first load when the scene graph is empty; vertical pans happen only on real level orlevelModechanges, not on every initial selection path.Exposes
window.__pascalCameraControlsunconditionally (dropsNODE_ENVgating that broke under Turbopack). Adds unit tests for framing logic and source wiring assertions.Reviewed by Cursor Bugbot for commit e7adddc. Bugbot is set up for automated code reviews on this repo. Configure here.